Skip to content

[WIP] Payjoin receiver#2011

Open
benalleng wants to merge 10 commits intowizardsardine:masterfrom
benalleng:payjoin-receiver
Open

[WIP] Payjoin receiver#2011
benalleng wants to merge 10 commits intowizardsardine:masterfrom
benalleng:payjoin-receiver

Conversation

@benalleng
Copy link
Copy Markdown

@benalleng benalleng commented Feb 10, 2026

This adds support for payjoin receive in liana.

This is currently WIP and has the following TODOs

  • Move bip21 to be created at runtime instead of at the time of receiver creation in lianad allowing us to not need to store bip21
  • Ensure that the expiry is properly handled as it currently panics when an incomplete session expires

@benalleng benalleng marked this pull request as ready for review March 12, 2026 18:06
Copy link
Copy Markdown

@xstoicunicornx xstoicunicornx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is still a WIP and my comments may be a bit premature but just wanted to brain dump some of the notes I took as part of the BOSS Payjoin Showcase session.

Some additional general comments:

  • should amount be added to URI?
  • should adjusting expiration will be added to settings?
  • when I ran this PR Liana panicked with this error when I tried to send a payjoin to the receiver:2026-04-09T16:19:40.194941Z ERROR liana_gui:82: panic occurred at line 508 of file lianad/src/database/sqlite/mod.rs: Some("database must be available: SqliteFailure(Error { code: Unknown, extended_code: 1 }, Some(\"table payjoin_outpoints has no column named added_at\"))") 0: backtrace::backtrace::libunwind::trace
  • in the GUI when payjoin is listed under previously generated addresses it shows the address rather than URI (I'm guessing this is related to this comment)

let bip21 = session.pj_uri().to_string();

let mut db_conn = self.db.connection();
db_conn.update_payjoin_receiver_bip21(new_index.into(), &bip21);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is now no-op this call is probably no longer needed?

Ok(GetAddressResult::new(address, new_index, Some(bip21)))
}

/// Get Payjoin URI (BIP21) and its sender/receiver status by txid
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Get Payjoin URI (BIP21) and its sender/receiver status by txid
/// Get receiver session ID and its sender/receiver status by txid

}

/// Get all active payjoin receiver sessions with their derivation indexes
pub fn get_active_payjoin_sessions(&self) -> Result<Vec<u32>, CommandError> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the plan is to also support sending payjoin in liana correct? Should this fn name maybe be get_active_receiver_payjoin_sessions instead to remove ambiguity in future when there may also be active payjoin sender sessions?

/* Payjoin OHttpKeys */
CREATE TABLE payjoin_ohttp_keys (
id INTEGER PRIMARY KEY NOT NULL,
relay_url TEXT UNIQUE NOT NULL,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be the payjoin directory url instead? Aren't OHttpKeys specific to a payjoin directory, not relay?

If sopayjoin_save_ohttp_keys and payjoin_get_ohttp_keys also would need to be updated.

}

/// Get bip21 for a receiver session by derivation index
pub fn get_payjoin_receiver_bip21(&mut self, derivation_index: u32) -> Option<String> {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to just return a normal address and not a bip21?

}
}

pub(crate) fn fetch_ohttp_keys(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to reuse PDK's implementation of fetch_ohttp_keys?

db_conn: &mut Box<dyn DatabaseConnection>,
secp: &secp256k1::Secp256k1<secp256k1::VerifyOnly>,
) -> Result<(), Box<dyn Error>> {
let proposal = proposal.apply_fee_range(None, None).save(persister)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No fee range actually being applied.

@benalleng
Copy link
Copy Markdown
Author

benalleng commented Apr 9, 2026

Thanks for the feedback,

Regarding the UI notes.

should amount be added to URI?

Since it is not a mandatory addition in the payjoin spec I thought that for a desktop on-chain only wallet the UX of adding an amount on the receive side is sort of unfamiliar, especially given the current liana receiver UI

should adjusting expiration will be added to settings?

I think again perhaps this is worth some user research but with the ability to do a fallback tx at any point I am not sure that extending the expiration as an option is great to put in the user's direct control

in the GUI when payjoin is listed under previously generated addresses it shows the address rather than URI (I'm guessing this is related to this comment)

This was something we landed on due wanting to discourage reuse of payjoin sessions. I could share this bip21 with as many people as I want similar to address reuse but instead of just bad privacy you would get a bunch of payjoin failures

The method again is a holdout as the last time I gave this some serious focus I transitioned from storing the bip21 to the session ID directly and I previously wanted to have access to the bip21 in the db where I just generate it in the gui now and don't store it directly

@benalleng
Copy link
Copy Markdown
Author

benalleng commented Apr 9, 2026

when I ran this PR Liana panicked with this error when I tried to send a payjoin to the receiver:2026-04-09T16:19:40.194941Z ERROR liana_gui:82: panic occurred at line 508 of file lianad/src/database/sqlite/mod.rs: Some("database must be available: SqliteFailure(Error { code: Unknown, extended_code: 1 }, Some("table payjoin_outpoints has no column named added_at"))") 0: backtrace::backtrace::libunwind::trace

This is definitely an artifact of me playing in the database between naming the timestamp added_at vs created_at https://github.com/wizardsardine/liana/pull/2011/changes#diff-4f33457fe83ad075401f91984457b7503d757068fcb26def0e090c584fd8439dR1146

@yashrajd
Copy link
Copy Markdown

should amount be added to URI?

Since it is not a mandatory addition in the payjoin spec I thought that for a desktop on-chain only wallet the UX of adding an amount on the receive side is sort of unfamiliar, especially given the current liana receiver UI

Familiarity might not be an issue IMO, since Liana users presumably also use other (esp mobile) wallets that have this. IMO it's more about the use case or user type: how often do they want to receive an specific exact amount? That said, this seems like a reasonable starting point.

should adjusting expiration will be added to settings?

I think again perhaps this is worth some user research but with the ability to do a fallback tx at any point I am not sure that extending the expiration as an option is great to put in the user's direct control

My hunch is, Liana users are likely to be sophisticated enough to use such a feature properly. Perhaps there could be appropriate messaging/guidance about using the feature.
From user research side, I'd say we have zero/little data to go on with, but you can always survey users to know their preferences (albeit with the "faster horse-carriages" caveat) (again without existing user base it is hard to rely on stated user preferences).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants